Fixes#11878
Instead of a confusing mix of django_auth_backed applying
ldap_to_django_username in its internals for one part of the
translation, and then custom logic for grabbing it from the email
attribute of the ldapuser in ZulipLDAPAuthBackend.get_or_build_user
for the second part of the translation,
we put all the logic in a single function user_email_from_ldapuser
which will be used by get_or_build of both ZulipLDAPUserPopulator and
ZulipLDAPAuthBackend.
This, building on the previous commits with the email search feature,
fixes the ldap sync bug from issue #11878.
If we can get upstream django-auth-ldap to merge
https://github.com/django-auth-ldap/django-auth-ldap/pull/154, we'll
be able to go back to using the version of ldap_to_django_username
that accepts a _LDAPUser object.
With this, django_to_ldap_username can take an email and find the ldap
username of the ldap user who has this email - if email search is
configured.
This allows successful authenticate() with ldap email and ldap password,
instead of ldap username. This is especially useful because when
a user wants to fetch their api key, the server attempts authenticate
with user_profile.email - and this used to fail if the user was an ldap
user (because the ldap username was required to authenticate
succesfully). See issue #9277.
This fixes a collection of bugs surrounding LDAP configurations A and
C (i.e. LDAP_APPEND_DOMAIN=None) with EmailAuthBackend also enabled.
The core problem was that our desired security model in that setting
of requiring LDAP authentication for accounts managed by LDAP was not
implementable without a way to
Now admins can configure an LDAPSearch query that will find if there
are users in LDAP that have the email address and
email_belongs_to_ldap() will take advantage of that - no longer
returning True in response to all requests and thus blocking email
backend authentication.
In the documentation, we describe this as mandatory configuration for
users (and likely will make it so soon in the code) because the
failure modes for this not being configured are confusing.
But making that change is pending work to improve the relevant error
messages.
Fixes#11715.
There are a few outstanding issues that we expect to resolve beforce
including this in a release, but this is good checkpoint to merge.
This PR is a collaboration with Tim Abbott.
Fixes#716.
If the social backend doesn't have get_verified_emails emails, and we
simply grab kwargs["details"].get("email") for the email, we should
still validate it is correct.
Needed for SAML. This will get covered by tests in upcoming commits that
add SAML support.
Previously, while Django code that relied on EXTERNAL_HOST and other
settings would know the Zulip server is actually on port 9991, the
upcoming Django SAML code in python-social-auth would end up detecting
a port of 9992 (the one the Django server is actually listening on).
We fix this using X-Forwarded-Port.
any_oauth_backend_enabled is all about whether we will have extra
buttons on the login/register pages for logging in with some non-native
backends (like Github, Google etc.). And this isn't about specifically
oauth backends, but generally "social" backends - that may not rely
specifically rely on Oauth. This will have more concrete relevance when
SAML authentication is added - which will be a "social" backend,
requiring an additional button, but not Oauth-based.
SOCIAL_AUTH_BACKEND / OAUTH_BACKEND_NAMES are currently the same
backends. All Oauth backends are social, and all social are oauth.
So we get rid of OAUTH_BACKEND_NAMES and use only SOCIAL_AUTH_BACKENDS.
Also cleans up the interface between the management command and the
LDAP backends code to not guess/recompute under what circumstances
what should be logged.
Co-authored-by: mateuszmandera <mateusz.mandera@protonmail.com>
The order of operations for our LDAP synchronization code wasn't
correct: We would run the code to sync avatars (etc.) even for
deactivated users.
Thanks to niels for the report.
Co-authored-by: mateuszmandera <mateusz.mandera@protonmail.com>
Fixes#13130.
django_auth_ldap doesn't give any other way of detecting that LDAPError
happened other than catching the signal it emits - so we have to
register a receiver. In the receiver we just raise our own Exception
which will properly propagate without being silenced by
django_auth_ldap. This will stop execution before the user gets
deactivated.
Fixes#9401.
This adds a FAKE_EMAIL_DOMAIN setting, which should be used if
EXTERNAL_HOST is not a valid domain, and something else is needed to
form bot and dummy user emails (if email visibility is turned off).
It defaults to EXTERNAL_HOST.
get_fake_email_domain() should be used to get this value. It validates
that it's correctly set - that it can be used to form valid emails.
If it's not set correctly, an exception is raised. This is the right
approach, because it's undesirable to have the server seemingly
peacefully operating with that setting misconfigured, as that could
mask some hidden sneaky bugs due to UserProfiles with invalid emails,
which would blow up the moment some code that does validate the emails
is called.
Previously, several of our URL patterns accidentally did not end with
`$`, and thus ended up controlling just the stated URL, but actually a
much broader set of URLs starting with it.
I did an audit and fixed what I believe are all instances of this URL
pattern behavior. In the process, I fixed a few tests that were
unintentionally relying on the behavior.
Fixes#13082.
In bf14a0af4, we refactored the Google authentication system to use
the same code as GitHub auth, but neglected to provide a
backwards-compatible URL available for use by older versions of the
mobile apps.
Fixes#13081.
This restructures the API endpoints that we currently have implemented
more or less for exclusive use by the mobile and desktop apps (things
like checking what authentication methods are supported) to use a
system that can be effectively parsed by our test_openapi
documentation.
This brings us close to being able to eliminate
`buggy_documentation_endpoints` as a persistently nonempty list.
For the emails that are associated to an existing account in an
organisation, the avatars will be displayed in the email selection
page. This includes avatar data in what is passed to the page.
Added `avatar_urls` to the context in `test_templates.py`.
Apparently GitHub changed the email address for these; we need to
update our code accordingly.
One cannot receive emails on the username@users.noreply.github.com, so
if someone tries creating an account with this email address, that
person would not be able to verify the account.
We were incorrectly setting LOCAL_UPLOADS_DIR to the empty string in
this code path, which would result in upload files being logged to the
root directory of the repository.
Fixes#12909.
Previous cleanups (mostly the removals of Python __future__ imports)
were done in a way that introduced leading newlines. Delete leading
newlines from all files, except static/assets/zulip-emoji/NOTICE,
which is a verbatim copy of the Apache 2.0 license.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
The current code looks like it's trying to redirect /integrations/doc/email
to /integrations when EMAIL_GATEWAY_PATTERN is not set.
I think it doesn't currently do this. The test for that pathway has a bug:
self.get_doc('integrations/doc-html/email', subdomain='zulip') needs a
leading slash, and putting the slash back in results in the test failing.
This redirection is not really desired behavior -- better is to
unconditionally show that the email integration exists, and just point the
user to https://zulip.readthedocs.io/en/latest/production/email-gateway.html
(this is done in a child commit).
This commit alone breaks things, needs to be merged with the follow-up
ones.
welcome-bot is removed from the explicit list, because it already is in
settings.INTERNAL_BOTS.
This feature is intended to cover all of our ways of exporting a
realm, not just the initial "public export" feature, so we should name
things appropriately for that goal.
Additionally, we don't want to include data exports in page_params;
the original implementation was actually buggy and would have.
Django’s default FileSystemFinder disallows STATICFILES_DIRS from
containing STATIC_ROOT (by raising an ImproperlyConfigured exception),
because STATIC_ROOT is supposed to be the result of collecting all the
static files in the project, not one of the potentially many sources
of static files.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This change serves to declutter webhook-errors.log, which is
filled with too many UnexpectedWebhookEventType exceptions.
Keeping UnexpectedWebhookEventType in zerver/lib/webhooks/common.py
led to a cyclic import when we tried to import the exception in
zerver/decorators.py, so this commit also moves this exception to
another appropriate module. Note that our webhooks still import
this exception via zerver/lib/webhooks/common.py.
This replaces the two custom Google authentication backends originally
written in 2012 with using the shared python-social-auth codebase that
we already use for the GitHub authentication backend. These are:
* GoogleMobileOauth2Backend, the ancient code path for mobile
authentication last used by the EOL original Zulip Android app.
* The `finish_google_oauth2` code path in zerver/views/auth.py, which
was the webapp (and modern mobile app) Google authentication code
path.
This change doesn't fix any known bugs; its main benefit is that we
get to remove hundreds of lines of security-sensitive semi-duplicated
code, replacing it with a widely trusted, high quality third-party
library.
The test_docs change is because Django runs test cases with DEBUG =
False, which ordinarily means it doesn’t serve /static during tests.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
The documentation suggests that you can get the dev server to use
production assets by setting PIPELINE_ENABLED = True, but that
resulted in Django being unable to find any static files because
FileSystemFinder was missing from STATICFILES_FINDERS. Using the
production storage configuration in this case reduces the number of
possible configurations and seems to result in things being less
broken.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
We create a path structure in the from:
`/var/<uuid>/test-backend/run_1234567/worker_N/`
A settings attribute, `TEST_WORKER_DIR`, was created as a worker's
directory for a given `test-backend` run's file storage. The
appropirate path is created in `setup_test_environment`, while each
workers subdirectory is created within `init_worker`.
This allows a test class to write to `settings.TEST_WORKER_DIR`,
populating the appropirate directory of a given worker. Also
providing the long-term approach to clean up filesystem access
in the backend unit tests.
We don’t need a hacked copy anymore. We run the installed version out
of node_modules in development, and a Webpack-bundled version of that
in production.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
In each url of urls.py, if we want to mark an endpoint as being
intentionally undocumented, then in the kwargs instead of directly
mapping like 'METHOD': 'zerver.views.package.foo', we can provide
a tag called 'intentionally_undocumented' and map like:
'METHOD': ('zerver.views.package.foo', {'intentionally_undocumented'}).
If an endpoint is marked as intentionally undocumented, but we find
some OpenAPI documentation for it then we'll throw an error, in which
case either the 'intentionally_undocumented' tag should be removed or
the faulty documentation should be removed.
This will allow us to mark a REQ variable as intentionally
undocumented. With this, we can remove some of the endpoints marked
as "buggy" even though they're not actually buggy, we just needed to
specify certain parameters as intentionally undocumented (e.g. the
stream_id for the /users/me/subscriptions/muted_topics endpoint.)
Any REQ variable with intentionally_undocumentated set to True
will not be added to the arguments_map data structure.
For some of the other "buggy" endpoints, we would want to mark the
entire endpoint as being undocumented intentionally via. the urls.py
file.
As of commit cff40c557b (#9300), these
files are no longer served directly to the browser. Disentangle them
from the static asset pipeline so we can refactor it without worrying
about them.
This has the side effect of eliminating the accidental duplication of
translation data via hash-naming in our release tarballs.
This reverts commit b546391f0b (#1148).
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
It appears not to have been useful and makes it marginally harder to
reason about how module resolution works. Paths to static content in
node_modules should be resolved through Webpack instead.
(This node_modules symlink was originally created in the pre-webpack world
where all of our static asset paths were based in static/.)
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
It's common for a broken settings.py file to result in Django not
starting, thus never writing to `/var/log/zulip/errors.log`. Such
behavior can be discouraging when the server 500s without a traceback
to accompany it. To fix this, we simply catch any exceptions in
django.setup(), if raised, and log the exception appropriately.
Comments tweaked by tabbott.
Closes: #7032.
Previously, our Github authentication backend just used the user's
primary email address associated with GitHub, which was a reasonable
default, but quite annoying for users who have several email addresses
associated with their GitHub account.
We fix this, by adding a new screen where users can select which of
their (verified) GitHub email addresses to use for authentication.
This is implemented using the "partial" feature of the
python-social-auth pipeline system.
Each email is displayed as a button. Clicking on that button chooses
the email. The email value is stored in a hidden input above the
button. The `primary_email` is displayed on top followed by
`verified_non_primary_emails`. Backend name is also passed as
`backend` to the template, which in our case is GitHub.
Fixes#9876.
This fixes an issue that caused LDAP synchronization to fail for
avatars. The problem occurred due to the lack of a 'name' attribute
on the BytesIO object that we pass to the upload backend (which is
only used in the S3 backend for computing Content-Type).
Fixes#12411.
The `AUTH_LDAP_ALWAYS_UPDATE_USER` is `True` by default, and this would sync the
attributes defined in the `AUTH_LDAP_USER_ATTR_MAP` to the user profile. But,
the default code in `django-auth-ldap` would work correctly only for `full_name`
field. This commit disables the setting by default, in favour of using the
`sync_ldap_user_data` script as a cron job.
Since positional arguments are interpreted differently by different
backends in Django's authentication backend system, it’s safer to
disallow them.
This had been the motivation for previously declaring the parameters
with default values when we were on Python 2, but that was not super
effective because Python has no rule against positional default
arguments and that convention for our authentication backends was
solely enforced by code review.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This commit adds a new developer tool: The "integrations dev panel"
which will serve as a replacement for the send_webhook_fixture_message
management command as a way to test integrations with much greater ease.
Jitsi Meet is the correct name for the product we integrate with. There is
one other reference to Jitsi, but it's in the db and will require a
migration.
This makes the implementation of `get_realm` consistent with its
declared return type of `Realm` rather than `Optional[Realm]`.
Fixes#12263.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
Fixes#12132.
Realm setting to disable avatar changes is already present.
The `AVATAR_CHANGES_DISABLED` setting now follows the same
2-setting model as `NAME_CHANGES_DISABLED`.
The github-services model for how GitHub would send requests to this
legacy integration is no longer available since earlier in 2019.
Removing this integration also allows us to finally remove
authenticated_api_view, the legacy authentication model from 2013 that
had been used for this integration (and other features long since
upgraded).
A few functions that were used by the Beanstalk webhook are moved into
that webhook's implementation directly.
An endpoint was created in zerver/views. Basic rate-limiting was
implemented using RealmAuditLog. The idea here is to simply log each
export event as a realm_exported event. The number of events
occurring in the time delta is checked to ensure that the weekly
limit is not exceeded.
The event is published to the 'deferred_work' queue processor to
prevent the export process from being killed after 60s.
Upon completion of the export the realm admin(s) are notified.
The main point here is that you should use a symlink rather than
changing it, since it's more maintenance work to update our nginx
configuration to use an alternative path than to just create a
symbolic link.
Fixes#12157.
Previously, we had some expensive-to-calculate keys in
zulip_default_context, especially around enabled authentication
backends, which in total were a significant contributor to the
performance of various logged-out pages. Now, these keys are only
computed for the login/registration pages where they are needed.
This is a moderate performance optimization for the loading time of
many logged-out pages.
Closes#11929.
These previously lived in Optional settings, which generally caused
users to not read it.
(Also do a bit of reorganization of the "optional settings" area).
Closes#2420
We add rate limiting (max X emails withing Y seconds per realm) to the
email mirror. By creating RateLimitedRealmMirror class, inheriting from
RateLimitedObject, and rate_limit_mirror_by_realm function, following a
mechanism used by rate_limit_user, we're able to have this
implementation mostly rely on the already existing, and proven over
time, rate_limiter.py code. The rules are configurable in settings.py in
RATE_LIMITING_MIRROR_REALM_RULES, analogically to RATE_LIMITING_RULES.
Rate limit verification happens in the MirrorWorker in
queue_processors.py. We don't rate limit missed message emails, as due
to using one time addresses, they're not a spam threat.
test_mirror_worker is adapted to the altered MirrorWorker code and a new
test - test_mirror_worker_rate_limiting is added in test_queue_worker.py
to provide coverage for these changes.
This avoids repeatedly calling a Django auth function that takes a few
hundred microseconds to run in auth_enabled_helper, which itself is
currently called 14 times in every request to pages using
common_context.
This renames references to user avatars, bot avatars, or organization
icons to profile pictures. The string in the UI are updated,
in addition to the help files, comments, and documentation. Actual
variable/function names, changelog entries, routes, and s3 buckets are
left as-is in order to avoid introducing bugs.
Fixes#11824.
Mateusz Mandera
Tim Abbott
Shikhar Varshney
Wyatt Hoodes
Anders Kaseorg
Alexandra Ciobica
neiljp (Neil Pilgrim)
Rishi Gupta
Eeshan Garg
Harshit Bansal
Hemanth V. Alluri
vinitS101
Anders Kaseorg
Shubham Padia
Vishnu Ks
Puneeth Chaganti
Mayank Madan
Shubham Padia
Ben Muschol