Commit Graph

418 Commits

Author SHA1 Message Date
Anders Kaseorg 365fe0b3d5 python: Sort imports with isort.
Fixes #2665.

Regenerated by tabbott with `lint --fix` after a rebase and change in
parameters.

Note from tabbott: In a few cases, this converts technical debt in the
form of unsorted imports into different technical debt in the form of
our largest files having very long, ugly import sequences at the
start.  I expect this change will increase pressure for us to split
those files, which isn't a bad thing.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2020-06-11 16:45:32 -07:00
Anders Kaseorg 69730a78cc python: Use trailing commas consistently.
Automatically generated by the following script, based on the output
of lint with flake8-comma:

import re
import sys

last_filename = None
last_row = None
lines = []

for msg in sys.stdin:
    m = re.match(
        r"\x1b\[35mflake8    \|\x1b\[0m \x1b\[1;31m(.+):(\d+):(\d+): (\w+)", msg
    )
    if m:
        filename, row_str, col_str, err = m.groups()
        row, col = int(row_str), int(col_str)

        if filename == last_filename:
            assert last_row != row
        else:
            if last_filename is not None:
                with open(last_filename, "w") as f:
                    f.writelines(lines)

            with open(filename) as f:
                lines = f.readlines()
            last_filename = filename
        last_row = row

        line = lines[row - 1]
        if err in ["C812", "C815"]:
            lines[row - 1] = line[: col - 1] + "," + line[col - 1 :]
        elif err in ["C819"]:
            assert line[col - 2] == ","
            lines[row - 1] = line[: col - 2] + line[col - 1 :].lstrip(" ")

if last_filename is not None:
    with open(last_filename, "w") as f:
        f.writelines(lines)

Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-06-11 16:04:12 -07:00
Anders Kaseorg 67e7a3631d python: Convert percent formatting to Python 3.6 f-strings.
Generated by pyupgrade --py36-plus.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2020-06-10 15:02:09 -07:00
Anders Kaseorg 6480deaf27 python: Convert more "".format to Python 3.6 f-strings.
Generated by pyupgrade --py36-plus --keep-percent-format, with more
restrictions patched out.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2020-06-10 14:48:09 -07:00
Dinesh 0c45e0118f tests: Clear remove/remock approach for ACCESS_TOKEN_URL endpoint.
ACCESS_TOKEN_URL works a different for apple authentication, so,
we removed and remocked the ACCESS_TOKEN_URL mock in
`register_extra_endpoints` override of apple auth test class.
It is cleaner to have it as generic feature of `social_auth_test`.

So, this commit adds a function that returns token_data_dict that
we had earlier and is called in the ACCESS_TOKEN_URL mock.
This function is overriden in apple auth test class to generate
payload of the format that apple auth expects.

Thanks to Mateusz Mandera for the simple idea.

Co-authored-by: Mateusz Mandera <mateusz.mandera@zulip.com>
2020-06-10 14:38:49 -07:00
Dinesh dc90d54b08 auth: Add Sign in with Apple support.
This implementation overrides some of PSA's internal backend
functions to handle `state` value with redis as the standard
way doesn't work because of apple sending required details
in the form of POST request.

Includes a mixin test class that'll be useful for testing
Native auth flow.

Thanks to Mateusz Mandera for the idea of using redis and
other important work on this.

Documentation rewritten by tabbott.

Co-authored-by: Mateusz Mandera <mateusz.mandera@zulip.com>
2020-06-09 17:29:35 -07:00
Dinesh d30f11888a logging: Set up a different logger for each backend.
Adds a top-level logger in `settings.LOGGING` `zulip.auth`
with the default handlers `DEFAULT_ZULIP_HANDLERS` and
an extra hanlder that writes to `/var/log/zulip/auth.log`.

Each auth backend uses it's own logger, `self.logger` which
is in form 'zulip.auth.<backend name>'.

This way it's clear which auth backend generated the log
and is easier to look for all authentication logs in one file.

Besides the above mentioned changes, `name` attribute is added to
`ZulipAuthMixin` so that these logging kind of calls wouldn't raise
any issues when logging is tried in a class without `name` attribute.

Also in the tests we use a new way to check if logger calls are made
i.e. we use `assertLogs` to test if something is logged.

Thanks to Mateusz Mandera for the idea of having a seperate logger
for auth backends and suggestion of using `assertLogs`.
2020-06-08 17:42:07 -07:00
Anders Kaseorg 8e4f22c184 auth: Require algorithms setting for JWT auth.
Calling jwt.decode without an algorithms list raises a
DeprecationWarning.  This is for protecting against
symmetric/asymmetric key confusion attacks.

This is a backwards-incompatible configuration change.

Fixes #15207.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2020-06-08 16:22:25 -07:00
Anders Kaseorg 8dd83228e7 python: Convert "".format to Python 3.6 f-strings.
Generated by pyupgrade --py36-plus --keep-percent-format, but with the
NamedTuple changes reverted (see commit
ba7906a3c6, #15132).

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2020-06-08 15:31:20 -07:00
Anders Kaseorg 139cb8026f auth: Accept next as POST parameter in POST requests.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2020-06-08 11:07:32 -07:00
Mateusz Mandera 3e7fc17788 auth: Delegate RemoteUser SSO to browser when using the desktop app. 2020-06-02 13:00:17 -07:00
Mateusz Mandera 90b2f933b0 tests: Change self.client_post to client_get in remote sso tests.
GET is the intended way to use this endpoint, this is how the mobile and
desktop apps pass their params.
2020-06-01 14:14:58 -07:00
whoodes cea7d713cd requirements: Upgrade boto to boto3.
Fixes: #3490

Contributors include:

Author:    whoodes <hoodesw@hawaii.edu>
Author:    zhoufeng1989 <zhoufengloop@gmail.com>
Author:    rht <rhtbot@protonmail.com>
2020-05-26 23:18:07 -07:00
Anders Kaseorg f5b33f9398 python: Further pyupgrade changes.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2020-05-26 11:43:40 -07:00
Anders Kaseorg 840cf4b885 requirements: Drop direct dependency on mock.
mock is just a backport of the standard library’s unittest.mock now.

The SAMLAuthBackendTest change is needed because
MagicMock.call_args.args wasn’t introduced until Python
3.8 (https://bugs.python.org/issue21269).

The PROVISION_VERSION bump is skipped because mock is still an
indirect dev requirement via moto.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2020-05-26 11:40:42 -07:00
Mateusz Mandera b66dc9de50 saml: Support IdP-initiated SSO. 2020-05-25 16:09:30 -07:00
Mateusz Mandera f2d052bff8 tests: Flush session before a simulated cross-domain POST in saml tests.
This is important, because lack of this meant that the POST request in
our tests still had the old session, with various params stored in it.
This mechanism doesn't work in reality in SAML, so the backend uses
redis to store and recover the params from redis. Without flushing the
session, these tests would fail to catch some breakages in the
redis-based mechanism.
2020-05-25 15:53:15 -07:00
Dinesh 288921d425 auth: Log when a user tries to login with deactivated account.
Helps to see if users are often trying to login with deactived
accounts.
A use case: Trackdown whether any deactivated bot users are still
trying to access the API.

This implementation adds a new key `inactive_user_id`
to `return_data` in the function `is_user_active` which
check if a `user_profile` is active. This reduces the effort
of getting `user_id` just before logging.

Modified tests for line coverage.
2020-05-24 17:27:19 -07:00
Mateusz Mandera dac4a7a70b saml: Figure out the idp from SAMLResponse.
Instead of plumbing the idp to /complete/saml/ through redis, it's much
more natural to just figure it out from the SAMLResponse, because the
information is there.
This is also a preparatory step for adding IdP-initiated sign in, for
which it is important for /complete/saml/ to be able to figure out which
IdP the request is coming from.
2020-05-24 16:40:28 -07:00
Mateusz Mandera c74f8363e2 saml: Gracefully handle bad SAMLResponses. 2020-05-24 16:40:28 -07:00
Mateusz Mandera 2f5fd272aa auth: Gracefully handle bad http responses from IdP in social auth.
If the IdP authentication API is flaky for some reason, it can return
bad http responses, which will raise HTTPError inside
python-social-auth. We don't want to generate a traceback
in those cases, but simply log the exception and fail gracefully.
2020-05-20 09:30:56 -07:00
Anders Kaseorg 8cdf2801f7 python: Convert more variable type annotations to Python 3.6 style.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2020-05-08 16:42:43 -07:00
Tim Abbott 3eaa71cef8 test_auth_backends: Add documentation for the main test interface. 2020-05-02 14:41:21 -07:00
Dinesh 5c1fe776c3 auth: Extend the template for "choose email" in GitHub auth flow.
This commit extends the template for "choose email" to mention for
users who have unverified emails that they need to verify them before
using them for Zulip authentication.

Also modified `social_auth_test_finish` to assert if all emails
are present in "choose email" screen as we need unverified emails
to be shown to user and verified emails to login/signup.

Fixes #12638 as this was the last task for that issue.
2020-05-02 14:30:31 -07:00
Mateusz Mandera 5f15af2382 tests: Clean out unnecessary ifs from GitHubAuthBackendTest helper.
After the refactor moving this logic into a helper inside of
GitHubAuthBackendTest, these checks became unnecessary and always True.
2020-05-02 13:40:29 -07:00
Dinesh 9f3872d2b4 tests: Refactor `social_auth_test`.
As "choose email" screen is only used for GitHub auth, the part
that deals with it is separated from `social_auth_test` and
dealt in a new function `social_auth_finish`. This new
`social_auth_finish` contains only the code that deals with
authentication backends that do not have "choose email" screen.
But it is overidden in GitHub test class to handle the
"choose email" screen.
It was refactored because `expect_choose_email_screen` blocks
were confusing while figuring out how tests work on non GitHub
auths.
2020-05-02 13:40:29 -07:00
Anders Kaseorg bdc365d0fe logging: Pass format arguments to logging.
https://docs.python.org/3/howto/logging.html#optimization

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2020-05-02 10:18:02 -07:00
Anders Kaseorg a552c2e5f9 auth: Use the clipboard instead of zulip:// for desktop auth flow.
This does not rely on the desktop app being able to register for the
zulip:// scheme (which is problematic with, for example, the AppImage
format).

It also is a better interface for managing changes to the system,
since the implementation exists almost entirely in the server/webapp
project.

This provides a smoother user experience, where the user doesn't need
to do the paste step, when combined with
https://github.com/zulip/zulip-desktop/pull/943.

Fixes #13613.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2020-04-30 16:45:00 -07:00
Mateusz Mandera f1ec02b40a auth: Add ExternalAuthResult to manage data in authentication flows.
This new type eliminates a bunch of messy code that previously
involved passing around long lists of mixed positional keyword and
arguments, instead using a consistent data object for communicating
about the state of an external authentication (constructed in
backends.py).

The result is a significantly more readable interface between
zproject/backends.py and zerver/views/auth.py, though likely more
could be done.

This has the side effect of renaming fields for internally passed
structures from name->full_name, next->redirect_to; this results in
most of the test codebase changes.

Modified by tabbott to add comments and collaboratively rewrite the
initialization logic.
2020-04-28 22:19:02 -07:00
Anders Kaseorg fead14951c python: Convert assignment type annotations to Python 3.6 style.
This commit was split by tabbott; this piece covers the vast majority
of files in Zulip, but excludes scripts/, tools/, and puppet/ to help
ensure we at least show the right error messages for Xenial systems.

We can likely further refine the remaining pieces with some testing.

Generated by com2ann, with whitespace fixes and various manual fixes
for runtime issues:

-    invoiced_through: Optional[LicenseLedger] = models.ForeignKey(
+    invoiced_through: Optional["LicenseLedger"] = models.ForeignKey(

-_apns_client: Optional[APNsClient] = None
+_apns_client: Optional["APNsClient"] = None

-    notifications_stream: Optional[Stream] = models.ForeignKey('Stream', related_name='+', null=True, blank=True, on_delete=CASCADE)
-    signup_notifications_stream: Optional[Stream] = models.ForeignKey('Stream', related_name='+', null=True, blank=True, on_delete=CASCADE)
+    notifications_stream: Optional["Stream"] = models.ForeignKey('Stream', related_name='+', null=True, blank=True, on_delete=CASCADE)
+    signup_notifications_stream: Optional["Stream"] = models.ForeignKey('Stream', related_name='+', null=True, blank=True, on_delete=CASCADE)

-    author: Optional[UserProfile] = models.ForeignKey('UserProfile', blank=True, null=True, on_delete=CASCADE)
+    author: Optional["UserProfile"] = models.ForeignKey('UserProfile', blank=True, null=True, on_delete=CASCADE)

-    bot_owner: Optional[UserProfile] = models.ForeignKey('self', null=True, on_delete=models.SET_NULL)
+    bot_owner: Optional["UserProfile"] = models.ForeignKey('self', null=True, on_delete=models.SET_NULL)

-    default_sending_stream: Optional[Stream] = models.ForeignKey('zerver.Stream', null=True, related_name='+', on_delete=CASCADE)
-    default_events_register_stream: Optional[Stream] = models.ForeignKey('zerver.Stream', null=True, related_name='+', on_delete=CASCADE)
+    default_sending_stream: Optional["Stream"] = models.ForeignKey('zerver.Stream', null=True, related_name='+', on_delete=CASCADE)
+    default_events_register_stream: Optional["Stream"] = models.ForeignKey('zerver.Stream', null=True, related_name='+', on_delete=CASCADE)

-descriptors_by_handler_id: Dict[int, ClientDescriptor] = {}
+descriptors_by_handler_id: Dict[int, "ClientDescriptor"] = {}

-worker_classes: Dict[str, Type[QueueProcessingWorker]] = {}
-queues: Dict[str, Dict[str, Type[QueueProcessingWorker]]] = {}
+worker_classes: Dict[str, Type["QueueProcessingWorker"]] = {}
+queues: Dict[str, Dict[str, Type["QueueProcessingWorker"]]] = {}

-AUTH_LDAP_REVERSE_EMAIL_SEARCH: Optional[LDAPSearch] = None
+AUTH_LDAP_REVERSE_EMAIL_SEARCH: Optional["LDAPSearch"] = None

Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-04-22 11:02:32 -07:00
Mateusz Mandera 62c0ab3f9d saml: Change which IdPs are returned to get_external_method_dicts.
If queried without a realm, get_external_method_dicts should only
have IdPs that can be used on all realms.
2020-04-21 13:49:34 -07:00
Hashir Sarwar e3b90a5ec8 api: Add a monotonic integer "feature level" for non-webapp clients.
The purpose is to provide a way for (non-webapp) clients,
like the mobile and terminal apps, to tell whether the
server it's talking to is new enough to support a given
API feature -- in particular a way that

* is finer-grained than release numbers, so that for
features developed after e.g. 2.1.0 we can use them
immediately on servers deployed from master (like
chat.zulip.org and zulipchat.com) without waiting the
months until a 2.2 release;

* is reliable, unlike e.g. looking at the number of
commits since a release;

* doesn't lead to a growing bag of named feature flags
which the server has to go on sending forever.

Tweaked by tabbott to extend the documentation.

Closes #14618.
2020-04-21 13:37:57 -07:00
Anders Kaseorg 5901e7ba7e python: Convert function type annotations to Python 3 style.
Generated by com2ann (slightly patched to avoid also converting
assignment type annotations, which require Python 3.6), followed by
some manual whitespace adjustment, and six fixes for runtime issues:

-    def __init__(self, token: Token, parent: Optional[Node]) -> None:
+    def __init__(self, token: Token, parent: "Optional[Node]") -> None:

-def main(options: argparse.Namespace) -> NoReturn:
+def main(options: argparse.Namespace) -> "NoReturn":

-def fetch_request(url: str, callback: Any, **kwargs: Any) -> Generator[Callable[..., Any], Any, None]:
+def fetch_request(url: str, callback: Any, **kwargs: Any) -> "Generator[Callable[..., Any], Any, None]":

-def assert_server_running(server: subprocess.Popen[bytes], log_file: Optional[str]) -> None:
+def assert_server_running(server: "subprocess.Popen[bytes]", log_file: Optional[str]) -> None:

-def server_is_up(server: subprocess.Popen[bytes], log_file: Optional[str]) -> bool:
+def server_is_up(server: "subprocess.Popen[bytes]", log_file: Optional[str]) -> bool:

-    method_kwarg_pairs: List[FuncKwargPair],
+    method_kwarg_pairs: "List[FuncKwargPair]",

Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-04-18 20:42:48 -07:00
Mateusz Mandera 7ed3c3f9f0 saml: Add setting to require limit_to_subdomains on configured IdPs.
If SAML_REQUIRE_LIMIT_TO_SUBDOMAINS is enabled, the configured IdPs will
be validated and cleaned up when the saml backend is initialized.
settings.py would be a tempting and more natural place to do this
perhaps, but in settings.py we don't do logging and we wouldn't be able
to write a test for it.
2020-04-16 17:04:12 -07:00
Mateusz Mandera 143db68422 saml: Implement limiting of IdP to specified realms.
Through the limit_to_subdomains setting on IdP dicts it's now possible
to limit the IdP to only allow authenticating to the specified realms.

Fixes #13340.
2020-04-16 17:04:08 -07:00
Mateusz Mandera 7a9d592dbe tests: Remove out-of-date comment on a saml test for multiple idps. 2020-04-16 17:02:36 -07:00
Anders Kaseorg c734bbd95d python: Modernize legacy Python 2 syntax with pyupgrade.
Generated by `pyupgrade --py3-plus --keep-percent-format` on all our
Python code except `zthumbor` and `zulip-ec2-configure-interfaces`,
followed by manual indentation fixes.

Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-04-09 16:43:22 -07:00
Mateusz Mandera 8686bbf637 auth: Show a user-facing page when wrong token given to /subdomain/.
This used to show a blank page. Considering that the links remain valid
only for 15 seconds it's important to show something more informative to
the user.
2020-04-05 12:29:09 -07:00
Stefan Weil d2fa058cc1
text: Fix some typos (most of them found and fixed by codespell).
Signed-off-by: Stefan Weil <sw@weilnetz.de>
2020-03-27 17:25:56 -07:00
Dinesh f526ae9377 tests: Change `is_signup` argument to boolean in `test_auth_backends.py`.
The function `prepare_login_url_and_headers` returns a register
link for any value of `is_signup` unless it's not none.
This commit changes it to a boolean for that function and other
functions using it so that it becomes much clearer when a
register link will be returned.

Also, all occurrences of `is_signup='1'` are changed to
`is_signup=True` to make the code consistent with the above change.
2020-03-25 15:59:37 -07:00
Mateusz Mandera 2c6b1fd575 rate_limit: Rename key_fragment() method to key(). 2020-03-22 18:42:35 -07:00
Mateusz Mandera 85df6201f6 rate_limit: Move functions called by external code to RateLimitedObject. 2020-03-22 18:42:35 -07:00
Dinesh 5cb476e03d auth: Handle confirm registration page in `stage_two_of_registration`.
When a user in login flow using github auth chooses a email that is
not associated with an existing account, it leads to a "continue to
registration" choice. This cannot be tested with the earlier version
of `stage_two_of_registration`.
Also added the test.
Thanks to Mateusz Mandera for the solution.

Co-authored-by: Mateusz Mandera <mateusz.mandera@protonmail.com>
2020-03-22 17:31:01 -07:00
Dinesh 3de646d2cf auth: Improve GitHub auth with multiple verified emails.
The previous model for GitHub authentication was as follows:

* If the user has only one verified email address, we'll generally just log them in to that account
* If the user has multiple verified email addresses, we will always
  prompt them to pick which one to use, with the one registered as
  "primary" in GitHub listed at the top.

This change fixes the situation for users going through a "login" flow
(not registration) where exactly one of the emails has an account in
the Zulip oragnization -- they should just be logged in.

Fixes part of #12638.
2020-03-22 17:31:01 -07:00
Dinesh 5888d7c0f5 auth: Change how config error URLs are configured.
URLs for config errors were configured seperately for each error
which is better handled by having error name as argument in URL.
A new view `config_error_view` is added containing context for
each error that returns `config_error` page with the relevant
context.
Also fixed tests and some views in `auth.py` to be consistent with
changes.
2020-03-22 17:15:18 -07:00
Steve Howell 1306239c16 tests: Use email/delivery_email more explicitly.
We try to use the correct variation of `email`
or `delivery_email`, even though in some
databases they are the same.

(To find the differences, I temporarily hacked
populate_db to use different values for email
and delivery_email, and reduced email visibility
in the zulip realm to admins only.)

In places where we want the "normal" realm
behavior of showing emails (and having `email`
be the same as `delivery_email`), we use
the new `reset_emails_in_zulip_realm` helper.

A couple random things:

    - I fixed any error messages that were leaking
      the wrong email

    - a test that claimed to rely on the order
      of emails no longer does (we sort user_ids
      instead)

    - we now use user_ids in some place where we used
      to use emails

    - for IRC mirrors I just punted and used
      `reset_emails_in_zulip_realm` in most places

    - for MIT-related tests, I didn't fix email
      vs. delivery_email unless it was obvious

I also explicitly reset the realm to a "normal"
realm for a couple tests that I frankly just didn't
have the energy to debug.  (Also, we do want some
coverage on the normal case, even though it is
"easier" for tests to pass if you mix up `email`
and `delivery_email`.)

In particular, I just reset data for the analytics
and corporate tests.
2020-03-19 16:04:03 -07:00
Steve Howell b1f8141200 tests: Prevent false positives for duplicate signups.
We specifically give the existing user different
delivery_email and email addresses, to prevent false
positives during the test that checks that users
signing up with an already-existing email get
an error message.

(We also rename the test.)
2020-03-19 14:32:18 -07:00
Mateusz Mandera f5e95c4fc1 requirements: Bump python-social-auth version.
We had a bunch of ugly hacks to monkey patch things due to upstream
being temporarily unmaintained and not merging PRs. Now the project is
active again and the fixes have been merged and included in the latest
version - so we clean up all that code.
2020-03-18 12:14:31 -07:00
Steve Howell 1b16693526 tests: Limit email-based logins.
We now have this API...

If you really just need to log in
and not do anything with the actual
user:

    self.login('hamlet')

If you're gonna use the user in the
rest of the test:

    hamlet = self.example_user('hamlet')
    self.login_user(hamlet)

If you are specifically testing
email/password logins (used only in 4 places):

    self.login_by_email(email, password)

And for failures uses this (used twice):

    self.assert_login_failure(email)
2020-03-11 17:10:22 -07:00
Steve Howell 00dc976379 tests: Use users for common_subscribe_to_streams.
We also use users for get_streams().
2020-03-11 14:18:29 -07:00