Replace default root logger with zulip.auth.apple for apple auth
in file zproject/backends.py and update the test cases
accordingly in file zerver/tests/test_auth_backends.py
Replaced mock.patch with assertLogs for testing log outputs
in file test_auth_backends.py.
This change requires adjusting
test_log_into_subdomain_when_email_is_none to use an explicit token
since that appears in the log output.
There are three functional side effects:
• Correct an insignificant but mathematically offensive bias toward
repeated characters in generate_api_key introduced in commit
47b4283c4b4c70ecde4d3c8de871c90ee2506d87; its entropy is increased
from 190.52864 bits to 190.53428 bits.
• Use the base32 alphabet in confirmation.models.generate_key; its
entropy is reduced from 124.07820 bits to the documented 120 bits, but
now it uses 1 syscall instead of 24.
• Use the base32 alphabet in get_bigbluebutton_url; its entropy is
reduced from 51.69925 bits to 50 bits, but now it uses 1 syscall
instead of 10.
(The base32 alphabet is A-Z 2-7. We could probably replace all of
these with plain secrets.token_urlsafe, since I expect most callers
can handle the full urlsafe_b64 alphabet A-Z a-z 0-9 - _ without
problems.)
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Our intent throughout the codebase is to treat email
case-insensitively.
The only codepath affected by this bug is remote_user_sso, as that's the
only one that currently passes potentially both a user_profile and
ExternalAuthDataDict when creating the ExternalAuthResult. That's why we
add a test specifically for that codepath.
The apple developer webapp consistently refers this App ID. So,
this clears any confusion that can occur.
Since python social auth only requires us to include App ID in
_AUDIENCE(a list), we do that in computed settings making it easier for
server admin and we make it much clear by having it set to
APP_ID instead of BUNDLE_ID.
Uses git release as this version 3.4.0 is not released to pypi.
This is required for removing some overriden functions of
apple auth backend class AppleAuthBackend.
With the update we also make following changes:
* Fix full name being populated as "None None".
c5c74f27dd that's included in update assigns first_name and last_name
to None when no name is provided by apple. Due to this our
code is filling return_data['full_name'] to 'None None'.
This commit fixes it by making first and last name strings empty.
* Remove decode_id_token override.
Python social auth merged the PR we sent including the changes
we made to decode_id_token function. So, now there is no
necessity for the override.
* Add _AUDIENCE setting in computed_settings.py.
`decode_id_token` is dependent on this setting.
In particular importing gitter data leads to having accounts with these
noreply github emails. We generally only want users to have emails that
we can actually send messages to, so we'll keep the old behavior of
disallowing sign up with such an email address. However, if an account
of this type already exists, we should allow the user to have access to
it.
A few major themes here:
- We remove short_name from UserProfile
and add the appropriate migration.
- We remove short_name from various
cache-related lists of fields.
- We allow import tools to continue to
write short_name to their export files,
and then we simply ignore the field
at import time.
- We change functions like do_create_user,
create_user_profile, etc.
- We keep short_name in the /json/bots
API. (It actually gets turned into
an email.)
- We don't modify our LDAP code much
here.
Due to authentication restrictions, a deployment may need to direct
traffic for mobile applications to an alternate uri to take advantage
alternate authentication mechansism. By default the standard realm URI
will be usedm but if overridden in the settings file, an alternate uri
can be substituted.
Because of other validation on these values, I don't believe any of
these does anything different, but these changes improve readability
and likely make GitHub's code scanners happy.
The helper should be used instead of constructing the dict manually.
Change get_account_data_dict, on GitHubAuthBackendTest
class, so it has a third argument, user_avatar_url.
This is a preparation for support using GitHub avatar
upon user resgistration (when the user logs using
GitHub).
subdomain=None didn't make much sense as a value, and wasn't actually in
use anywhere, except one test where it was accidental. All tests specify
the subdomain explicitly, so we should change the type to str, and make
it an obligatory kwarg.
Adds the ability to set a SAML attribute which contains a
list of subdomains the user is allowed to access. This allows a Zulip
server with multiple organizations to filter using SAML attributes
which organization each user can access.
Cleaned up and adapted by Mateusz Mandera to fit our conventions and
needs more.
Co-authored-by: Mateusz Mandera <mateusz.mandera@zulip.com>
Old: a validator returns None on success and returns an error string
on error.
New: a validator returns the validated value on success and raises
ValidationError on error.
This allows mypy to catch mismatches between the annotated type of a
REQ parameter and the type that the validator actually validates.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
The most import change here is the one in maybe_send_to_registration
codepath, as the insufficient validation there could lead to fetching
an expired PreregistrationUser that was invited as an administrator
admin even years ago, leading to this registration ending up in the
new user being a realm administrator.
Combined with the buggy migration in
0198_preregistrationuser_invited_as.py, this led to users incorrectly
joining as organizations administrators by accident. But even without
that bug, this issue could have allowed a user who was invited as an
administrator but then had that invitation expire and then joined via
social authentication incorrectly join as an organization administrator.
The second change is in ConfirmationEmailWorker, where this wasn't a
security problem, but if the server was stopped for long enough, with
some invites to send out email for in the queue, then after starting it
up again, the queue worker would send out emails for invites that
had already expired.
Fixes#14828.
Giving the /subdomain/<token>/ url there could feel buggy if the user
ended up using the token in the desktop app, and then tried clicking the
"continue in browser" link - which had the same token that would now be
expired. It's sufficient to simply link to /login/ instead.
Overrides some of internal functions of python-social-auth
to handle native flow.
Credits to Mateusz Mandera for the overridden functions.
Co-authored-by: Mateusz Mandera <mateusz.mandera@zulip.com>
Use read-only types (List ↦ Sequence, Dict ↦ Mapping, Set ↦
AbstractSet) to guard against accidental mutation of the default
value.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
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>
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>
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>
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>
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`.
Anders Kaseorg
palash
Mateusz Mandera
Dinesh
Mohit Gupta
Tim Abbott
Steve Howell
Tim April
Clara Dantas
Brainrecursion
sahil839
Anders Kaseorg