Fellow Report - September 23, 2017

[Tim Graham]

Triaged

---

https://code.djangoproject.com/ticket/28611 - Redirects contrib app - 
CharField(max_length=200) is too little (duplicate)

Authored

--

https://github.com/jazzband/django-hosts/pull/73 - Drop support for Django 
< 1.11 and add support for 2.0

Reviewed/committed

--

https://github.com/django/django/pull/7611 - Fixed #26608 -- Add support 
for OVER clause (window expressions).

https://github.com/django/django/pull/6385 - Fixed #14370 -- Allowed using 
a Select2 widget for ForeignKey and ManyToManyField in the admin.

https://github.com/django/django/pull/9096 - Fixed #28597 -- Fixed crash 
with the name of a model's autogenerated primary key in an Index's fields.

https://github.com/django/django/pull/9100 - Fixed #28488 -- Reallowed 
error handlers to access CSRF tokens.

https://github.com/django/django/pull/9107 - Fixed #28576 -- Added color 
interpretation method to GDALBand.

https://github.com/django/django/pull/9086 - Refs #28595 -- Added a hook to 
add execute wrappers for database queries.
https://github.com/django/django/pull/9081 - Fixed #27332 -- Added 
FilteredRelation API for conditional join (ON clause) support.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/87218bda-abc0-49c9-a4e8-7d1b99e25125%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: PostgreSQL aggregation and views through unmanaged models

[Dylan Young]

Is there any reason not to query the models on startup to determine which 
ones support the optimization and which do not (naively, which are proper 
tables and which are views, but perhaps postgresql can provide the 
information instead of relying on heuristics which could change in any 
release)?  Not only would this solve this issue, it would add a general 
mechanism for auto-configuring optimizations based on underlying 
model/table characteristics, essential for any ORM IMO. 

On Monday, 22 May 2017 00:05:38 UTC-3, charettes wrote:
>
> Hello fellow developers,
>
> As some of you may know PostgreSQL 9.1 added support for GROUP'ing BY
> selected table primary keys[0] only. Five years ago it was reported[1] that
> Django could rely on this feature to speed up aggregation on models backed
> up by tables with either many fields or a few large ones.
>
> Being affected by this slow down myself I decided to dive into the ORM 
> internals
> and managed to get a patch that made it in 1.9[2] thanks to Anssi's and 
> Josh's
> review[3].
>
> One subtle thing I didn't know back in the time is that PostgreSQL query 
> planner
> isn't able to introspect database views columns' functional dependency 
> like it
> does with tables and thus prevents the primary key GROUP'ing optimization 
> from
> being used.
>
> While Django doesn't support database views officially it documents that
> unmanaged models can be used to query them[4] and thereby perform 
> aggregation on
> them and generating an invalid query.
>
> This was initially reported as a crashing bug 9 months ago[5] and the 
> consensus
> at this time was that it was an esoteric edge case since there was few 
> reports
> of breakages and it went off my radar. Fast-forward to a month ago, this is
> reported again[6] and it takes the reporter quite a lot of effort to 
> determine
> the origin of the issue, pushing me to come up with a solution as I 
> introduced
> this behavior.
>
> Before Claude makes me realize this is a duplicate of the former report 
> (which I
> completely forgot about in the mean time) I implement a patch and commit 
> it once
> it's reviewed [7].
>
> When I closed the initial ticket as "fixed" the reporter brought to my 
> attention
> that this was now introducing a performance regression for unmanaged models
> relying on aggregation and that we should document how to disable this
> optimization by creating a backend subclass as a workaround instead.
>
> In my opinion the current situation is as follow. The optimization 
> introduced a
> break in backward compatibility in 1.9 as we've always documented that 
> database
> views could be queried against using unmanaged models. If this issue had 
> been
> discovered during the 1.9 release cycle it would have been eligible for a
> backport because it was a bug in a newly introduced feature. Turning this
> optimization off for unmanaged models by assuming they could be views is 
> only
> going to degrade performance of queries using unmanaged models to perform
> aggregation on tables with either a large number of columns or large 
> columns
> using PostgreSQL.
>
> Therefore I'd favor we keep the current adjustment in the master branch as 
> it
> restores backward compatibility but I don't have strong feelings about 
> reverting
> it either if it's deemed inappropriate.
>
> Another solution I came up while writing this post would be to replace the
> feature flag by a callable that takes a model as a single parameter and 
> returns
> whether or not the optimization can be performed against it. The default
> implementation would return `mode._meta.managed` but it would make it 
> easier for
> users affected by this to override in order to opt-in or out based on their
> application logic.
>
> Thank you for your time,
> Simon
>
> [0] https://www.postgresql.org/docs/9.1/static/sql-select.html#SQL-GROUPBY
> [1] https://code.djangoproject.com/ticket/19259
> [2] 
> https://github.com/django/django/commit/dc27f3ee0c3eb9bb17d6cb764788eeaf73a371d7
> [3] https://github.com/django/django/pull/4397
> [4] https://docs.djangoproject.com/en/1.11/ref/models/options/#managed
> [5] https://code.djangoproject.com/ticket/27241
> [6] https://code.djangoproject.com/ticket/28107
> [7] 
> https://github.com/django/django/commit/daf2bd3efe53cbfc1c9fd00222b8315708023792
>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/64ee2617-76c0-44a1-8b45-59ef48f61ae4%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: New Feature: Allow password reset token to expire in under a day

[Luke Plant]

Hi Zach,

To be clear, I'm not totally opposed to the change. There could 
certainly be advantages to updating the code, especially if we move to 
TimestampSigner.


However, from a quick look, I'm not sure if TimestampSigner will be 
possible - we want the hashed value to incorporate some internal data, 
but don't want that internal data to be part of the signed message that 
Signer produces (for various reasons, including the fact that we want to 
keep the URL as short as possible for maximum compatibility). I think we 
should also be careful not to change the public interface of 
PasswordResetTokenGenerator, since people may have subclassed it, which 
limits the amount of cleanup you can do here.


I imagine that changing the timestamp to support better precision will 
also create work for people upgrading (the setting change, plus URLconf 
changes).


What I am suggesting is that we should seriously consider whether all 
this is worth it given the extremely marginal security benefits.


Regards,

Luke


On 23/09/17 04:33, Zhiqiang Liu wrote:

Luke,

thanks for the long explanation. I see your points here. I actually 
saw the make token function and was thinking about it what is the best 
way to do with that. I think most people here feel there's need to at 
least allow some flexibility for the time out since there will be 
cases under a day is needed.
I will keep this discussion for a couple of more days to see if we can 
get consensus and how we should implemented if needed.


Zach

On Friday, September 22, 2017 at 3:04:01 PM UTC-4, Luke Plant wrote:

I would be +1 to what Adam wrote from me i.e. just allow the value
to accept floats.

However, I don't think it will work due to the way that we round
the precision of timestamps to days

.
This was done partly to reduce the number of characters needed to
express the timestamp, to keep URLs as short as possible. We would
have to change the mechanism to store more precision into the
timestamp. This would result in an upgrade 'bump' for users (i.e.
links generated before the upgrade would become invalid after
upgrade).

However, I really question whether we need any change here, and
whether it would be a good idea.

Having a short expiration time (less than 1 hour) could cause
major problems for some people - plenty of systems introduce 5 or
10 minute delays in mail delivery, and with some people's internet
connection it can take several minutes to open a web page. This
also means that some people end up finishing the process of
whatever they were doing the next day (I know I've done this
several times on various sites), so a timeout of at least 1 or 2
days is a good default. If you want to come back after the weekend
and carry on, 3 days makes more sense as a minimum.

In terms of security, I don't think there is really any need for
anyone to reduce below the default at all (see below). So I'm very
unconvinced about the need for changing to PASSWORD_RESET_TIMEOUT
- it is just unnecessary upgrade work for some existing projects
(this is the biggest consideration for me), and it could encourage
people to set the value to something low that would decrease
usability.

*Security:*

The security of the password reset feature is almost entirely
independent of the value of the timeout setting. There are 3
attack vectors I can see:

1) Someone's email account is compromised, and they then do a
password reset on a Django site.

We simply can't protect against this AFAICS.

2) Someone's email account is compromised, and they find/use a
password reset email in the person's inbox.

This is the only scenario for which having a shorter timeout makes
a difference. It is somewhat unlikely, because in 99% of cases the
attacker would be able to generate a password reset email
themselves after compromising the account. For this narrow case
where the attacker is unwilling/unable to trigger/receive a new
password reset email, it is worth having some protection against
them being able to use old tokens, but 3 days seems plenty short
enough for this situation, especially given the fact that a *used*
password reset token immediately becomes invalid due to the way we
hash on internal state of the user record.

3) A brute force attack.

To do this, the attacker has to:

1. Supply a user ID (let's assume this is easy)

2. ***Choose*** a timestamp (very easy, just choose the current time)

3. Create a 20 character hexadecimal hmac that matches both the
timestamp and the internal state of the user (see
https://github.com/django/django/blob/master/django/contrib/auth/tokens.py

).

Since the attacker 

Need help with solving ticket #28619

[Heba Khan]

Hi everyone! 

I want to contribute to Django as part of my minor project that is a part 
of my curriculum. I encountered ticket 
#28619: https://code.djangoproject.com/ticket/28619 
Please advise me on how to move ahead with this problem and fix it.

Thank you very much in advance. 

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/15e9f8b6-d2e8-4e92-9ba8-7f8d991e12aa%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.